TypeCombinator returns non-empty-array for union of isIterableAtLeastOnce()->yes()#3937
Conversation
| } | ||
|
|
||
| if (TrinaryLogic::createYes()->and(...$isIterableAtLeastOnce)->yes()) { | ||
| $commonAccessoryTypes[] = new NonEmptyArrayType(); |
There was a problem hiding this comment.
atm this can lead to some arrays, which - while processing - have the NonEmptyArrayType 2-times.
when TypeCombinator::union() finally ends, this "duplication" gets normalized away.
we could check at this IF-condition here, whether $commonAccessoryTypes already contains NonEmptyArrayType so we don't add it a 2nd time.
for readability I have chosen not todo this additionaly check. but I am fine with adding it, in case you think we should do it.
|
This pull request has been marked as ready for review. |
staabm
force-pushed
the
non-empty-offsets
branch
from
c9ef256 to
492f775
Compare
|
I'd say this isn't necessary. Intersection type of ArrayType and HasOffsetValueType should already know it's iterable at least once (it's non-empty). The question is - why the tests you changed already don't result in |
before this PR the see phpstan-src/src/Type/TypeCombinator.php Lines 687 to 704 in 5df4617 therefore in e.g. |
|
Ohhh, I get it, this makes sense then! |
|
Thank you! |
3 participants
a array can be non-empty caused by different accessory types, namely
NonEmptyArrayType,HasOffsetType,HasOffsetValueType.before this PR we only returned a non-empty-array result from
TypeCombinator::union()when all elements in the union were using the same accessory types, e.g.union(array&non-empty, array&non-empty)union(array&hasOffset('a'), array&hasOffset('a'))after this PR we in addition return a non-empty-array for unions of arrays which are non-empty caused by different accessories, e.g.
union(array&non-empty, array&hasOffset('a'))union(array&hasOffset('b'), array&hasOffset('a'))union(array&hasOffset('b'), array&hasOffsetValue(4, 'a'))refs #3924 (comment)